fix: check quoted text for content safety#9232
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the content safety check stage to extract and verify quoted text from reply components when no explicit check text is provided, and adds corresponding unit tests. A potential issue was identified where the variables ok and info are defined inside a loop over texts; if texts is empty, this could lead to a NameError when they are accessed after the loop. Initializing these variables before the loop is recommended.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| for text in texts: | ||
| ok, info = self.strategy_selector.check(text) | ||
| if not ok: | ||
| break | ||
| if not ok: |
There was a problem hiding this comment.
The variables ok and info are defined inside the for text in texts: loop. If texts is empty (or from the perspective of static type checkers like mypy or pyright which cannot guarantee the list is non-empty), these variables will be unbound when accessed on line 48 and line 57, leading to a potential NameError or type-checking failures.
Initializing ok and info before the loop resolves this issue and makes the code more robust.
ok, info = True, ""
for text in texts:
ok, info = self.strategy_selector.check(text)
if not ok:
break
if not ok:There was a problem hiding this comment.
Addressed in 24b1d1a by initializing ok and info before the loop. Repository-wide Ruff checks pass, along with the related regression suite (121 passed). Thank you.
|
/gemini Please review the latest changes. Commit 24b1d1a addresses the unbound-variable feedback from the previous review. |
There was a problem hiding this comment.
Code Review
This pull request enhances the content safety check stage to also inspect quoted text within reply components of incoming messages. It introduces the ReplyChainParser to extract text from replies and iterates through all extracted texts to perform safety checks. Additionally, unit tests have been added to verify this behavior. The review feedback suggests skipping empty or whitespace-only strings during the safety check loop to prevent redundant external API calls and reduce latency.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| ok, info = True, "" | ||
| for text in texts: | ||
| ok, info = self.strategy_selector.check(text) | ||
| if not ok: | ||
| break |
There was a problem hiding this comment.
If texts contains empty or whitespace-only strings (for example, if the user sends an image/file without text, or if check_text is passed as an empty string), calling self.strategy_selector.check(text) will still execute. If external content safety APIs (like Baidu AIP) are enabled, this will result in unnecessary network requests, which increases latency and can incur API usage costs.
We should skip empty or whitespace-only strings in the loop to avoid these redundant checks.
| ok, info = True, "" | |
| for text in texts: | |
| ok, info = self.strategy_selector.check(text) | |
| if not ok: | |
| break | |
| ok, info = True, "" | |
| for text in texts: | |
| if not text or not text.strip(): | |
| continue | |
| ok, info = self.strategy_selector.check(text) | |
| if not ok: | |
| break |
| texts = [check_text] | ||
|
|
||
| ok, info = True, "" | ||
| for text in texts: |
There was a problem hiding this comment.
hi, join these text parts and check once is enough
dosubot
Bot
added
size:S
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
2 participants
Summary
ReplyChainParserfor both embedded chains andmessage_strRoot cause
The content safety stage only checked
event.get_message_str(), while quoted text was stored inReplycomponents and later added to the LLM request. This allowed blocked keywords to reach the model when they appeared only in a quoted message.Quoted text is checked separately from the current message so regular-expression anchors such as
^...$keep their existing semantics.Closes #9227.
Validation
uv run ruff format --check .uv run ruff check .uv run pytest -q tests/test_content_safety_check.py tests/test_quoted_message_parser.py tests/unit/test_aiocqhttp_reply.py tests/unit/test_astr_message_event.py tests/test_smoke.py(121 passed)The complete test suite was also attempted, but the unrelated existing test
test_dify_image_upload_uses_media_resolver_for_data_urlhangs when run in isolation in this environment.Summary by Sourcery
Validate inbound content safety against both the main message text and any quoted reply text to prevent blocked keywords from reaching the model.
Bug Fixes:
Enhancements:
Tests: